Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Redox Dockerfile and Travis Environment #43303

Merged
merged 5 commits into from
Aug 15, 2017

Conversation

jackpot51
Copy link
Contributor

This adds Redox to the Travis build. This is an example implementation of #43206

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@aidanhs
Copy link
Member

aidanhs commented Jul 17, 2017

As noted in the referenced issue, this likely won't be merged imminently if only because we're at the builder capacity.

That said, there is a disabled folder in https://github.com/rust-lang/rust/tree/master/src/ci/docker/disabled which I could see us accepting a PR for in the meantime.

@jackpot51
Copy link
Contributor Author

jackpot51 commented Jul 18, 2017

@aidanhs You could disable another platform. Might I suggest s390x-linux?

@Mark-Simulacrum
Copy link
Member

I'm against removing anything from Tier 2 support that's there already today: "Official binary releases are provided for the platform." is the guarantee on the forge, and I think that implies that we'll continue producing those. I've heard that we expect to increase the quantity of available builders soon (though I don't know of a specific timeframe) which would allow us to add redox easily.

@jackpot51
Copy link
Contributor Author

@Mark-Simulacrum I have resources to build this, is it possible to share them with you?

@aidanhs
Copy link
Member

aidanhs commented Jul 18, 2017

@jackpot51 previously builders have been added in an ad-hoc manner effectively up to the limit of current funding. At that point there was a realisation that a more sustainable model needs developing, which is what @aturon was talking about in the linked issue. While this was in progress, we have had to postpone other targets like #41133 and #41149 - this is a known ongoing problem!

My interpretation of what @aturon was saying in the linked issue is that new builders will arrive at some point and it would probably be more reasonable to talk about adding redox as a builder then.

Until that point, we probably want to avoid more ad-hoc changes - disabling existing builders likely falls under that category! (and, even if we did, we're so close to capacity that we may still not be comfortable with adding another builder since we're already seeing adverse effects of limited builders in other ways than just PRs on this repo).

@aturon or @alexcrichton may want to step in here or on the linked issue with any corrections to the above/other thoughts.

@aidanhs
Copy link
Member

aidanhs commented Jul 18, 2017

@jackpot51 I'm not aware of a way for us accept external resources right now, but we can certainly discuss it in the next infra team meeting since it's probably something we'd like to enable.

@aidanhs aidanhs added I-nominated T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 18, 2017
@alexcrichton
Copy link
Member

@aidanhs nah I think you're spot on! We're currently exploring various methods of expanding capacity, but we haven't got something concrete just yet. Additionally yes, while we'd like to support custom hardware we don't currently have the infrastructure to do so.

@arielb1 arielb1 added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jul 18, 2017
@bors
Copy link
Contributor

bors commented Jul 19, 2017

☔ The latest upstream changes (presumably #43316) made this pull request unmergeable. Please resolve the merge conflicts.

@aidanhs
Copy link
Member

aidanhs commented Aug 3, 2017

The core and infra team have made some progress talking about this, particularly for builders that just want a stdlib. We're not there yet, but it is something we're working on.

Just to double check - am I correct in thinking that "just want a stdlib" is what you're looking for here?

@jackpot51
Copy link
Contributor Author

@aidanhs for now, that is correct

@alexcrichton
Copy link
Member

Thanks for your patience here @jackpot51! Lots of teams have had a lot of discussion here, but the highlights here are:

  • Infrastructure-wise there's a pretty big difference between "std nightlies" and "rustc nightlies". For producing the standard library it's actually a very small amount of resource investment on our end, and we can have one builder creating a whole bunch of standard libraries. For example right now the cross builder builds a number of standard libraries for various architectures.
  • Investment-wise, because a standard library is pretty cheap, Mozilla, the current organization footing the bill, is willing to foot the bill for one builder doing a whole bunch of standard librararies. Basically if all you need is libstd in nightlies, we've got your back capacity-wise!
  • Finally, the libs team discussed the maintenance burden with the standard library here, and sort of about the "what tier is this in" question. Our conclusions were somewhat vague, but intentionally so. We can't, at this time, provide a hard guarantee about whether we will forever be able to gate on platforms like redox building on CI (e.g. gating all changes to rust-lang/rust on the redox platform). What we can say, however, is that for the near future we're totally OK with gating on redox. If we add new features to the standard library which require bindings for redox we'll likely just stub it out returning an error, and then y'all will come along later to implement it!

So tl;dr we can merge this patch if you'd like by moving the new builder here into the existing cross builder. Does all that sound ok to you?

@jackpot51
Copy link
Contributor Author

@alexcrichton Yes, that all sounds good to me! Do you want me to modify the PR or will you?

@alexcrichton
Copy link
Member

@jackpot51 oh if you could take care of it that'd be great, I have a feeling you can get it done much more quickly than I can!

@aidanhs aidanhs added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). I-nominated labels Aug 10, 2017
@jackpot51
Copy link
Contributor Author

@alexcrichton @aidanhs I have rebased and I have added Redox to the cross build. I removed the change that makes travis build dist-x86_64-redox, but I left in the Dockerfile for future usage.

@@ -0,0 +1,42 @@
FROM ubuntu:16.04
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is now equivalent to the cross container, right? If not, can you edit the scripts here to use the shared scripts in the src/ci/docker/shared folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean src/docker/scripts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This folder for shared scripts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry I also forgot to mention, but can this be moved to the src/ci/docker/disabled folder?

@jackpot51
Copy link
Contributor Author

@alexcrichton it should be simpler now. Since the toolchain installation is the same, the script in cross is used by the dist-x86_64-redox dockerfile

@carols10cents carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 14, 2017
@carols10cents
Copy link
Member

friendly ping @alexcrichton, this looks ready for you!

@alexcrichton
Copy link
Member

Ah sorry should have commented more prominently, bu the dist-x86_64-redox builder should move to the disabled folder

@jackpot51
Copy link
Contributor Author

Ok, will do

@jackpot51
Copy link
Contributor Author

@alexcrichton it is done

@alexcrichton
Copy link
Member

@bors: r+

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 14, 2017

📌 Commit 1cc33c0 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Aug 15, 2017

⌛ Testing commit 1cc33c0 with merge 69e6a7ff17a5c84e98f099832d2d5a40c2ae16e5...

@bors
Copy link
Contributor

bors commented Aug 15, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Aug 15, 2017

Legit. Please try to chmod a+x src/ci/docker/cross/install-x86_64-redox.sh and commit again.

[00:00:48] Step 15/32 : RUN ./install-x86_64-redox.sh
[00:00:48]  ---> Running in 3ba43dab0982
[00:00:48] /bin/sh: 1: ./install-x86_64-redox.sh: Permission denied
[00:00:49] The command '/bin/sh -c ./install-x86_64-redox.sh' returned a non-zero code: 126
[00:00:49] The command has failed after 5 attempts.

@jackpot51
Copy link
Contributor Author

Oops

@jackpot51
Copy link
Contributor Author

@kennytm @alexcrichton That script is marked executable in git now.

@aidanhs
Copy link
Member

aidanhs commented Aug 15, 2017

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Aug 15, 2017

📌 Commit 727b473 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Aug 15, 2017

⌛ Testing commit 727b473 with merge 1b08e0f...

bors added a commit that referenced this pull request Aug 15, 2017
Add Redox Dockerfile and Travis Environment

This adds Redox to the Travis build. This is an example implementation of #43206
@bors
Copy link
Contributor

bors commented Aug 15, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 1b08e0f to master...

@bors bors merged commit 727b473 into rust-lang:master Aug 15, 2017
@jackpot51
Copy link
Contributor Author

🎉 🎉 🍰 🍰 🎆 🎆

@jackpot51
Copy link
Contributor Author

@alexcrichton Redox rust-std is not present here: https://static.rust-lang.org/dist/nightly-channel.toml

Is there something I am missing?

@jackpot51
Copy link
Contributor Author

Fixed here: #43950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants